- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3
adding FromSetting for declarative spider param defaults #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
adding FromSetting for declarative spider param defaults #25
Conversation
| raise | ||
| super().__init__(*args, **kwargs) | ||
|  | ||
| def _set_crawler(self, crawler): | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, where is this called (normally)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is called when a spider is created using from_crawler()
https://github.com/scrapy/scrapy/blob/f2531808f3b0041786c68e50a88f6c5500a37dd4/scrapy/spiders/__init__.py#L76
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh.
I think this should override from_crawler() instead, though I'm not 100% sure if it's possible.
| Regarding test failures, it's an incompatibility with Scrapy 2.13 which I haven't thought of checking, either adding  And it should be unrelated to the changes of this PR. | 
| I added a conftest.py to make it pass the tests locally. The file content is: Should I add this change to the PR? | 
| Yes please. | 
| Though I don't think touching the  | 
| Codecov Report❌ Patch coverage is  
 Additional details and impacted files@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
- Coverage   85.71%   83.45%   -2.27%     
==========================================
  Files           4        5       +1     
  Lines         105      139      +34     
  Branches       19       25       +6     
==========================================
+ Hits           90      116      +26     
- Misses         11       16       +5     
- Partials        4        7       +3     ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
 | 
This change introduces FromSetting, a marker that allows spider parameters to declare defaults pulled directly from crawler.settings.
Defaults are resolved in _set_crawler following this precedence:
Unit tests were added to ensure correct precedence and proper integration with Scrapy settings.